-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for issues found by SpotBugs #1283
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1283 +/- ##
===============================================
+ Coverage 67.495% 67.501% +0.006%
+ Complexity 8150 8149 -1
===============================================
Files 558 557 -1
Lines 33364 33352 -12
Branches 5608 5607 -1
===============================================
- Hits 22519 22513 -6
+ Misses 8657 8651 -6
Partials 2188 2188
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - requested a couple of tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of minor changes requested, otherwise looks good once these are addressed and tests pass again.
rm ExternalBlock class, add static factory method - resolves use of initialized value found by SpotBugs rm unused nextRecord nextRecord -> samRecord basic content ID check test createRawNonExternalBlock and more tests enableFileSourceTest revert CRAMFileReaderTest BlockTest to Block package Update src/test/java/htsjdk/samtools/cram/structure/BlockTest.java Co-Authored-By: jmthibault79 <[email protected]>
e1dbbec
to
71aee8c
Compare
Description
Resolves #1279 - removed the ExternalBlock class which was not truly necessary
Resolves #1280 - removed the unused
CramIterator.nextRecord
fieldResolves #1281 - removed the unused
CramHeader.clone()
methodChecklist